Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip OAuth and use basic auth #1011

Closed
wants to merge 1 commit into from

Conversation

MichalKarol
Copy link
Collaborator

No description provided.

@ad-m
Copy link
Member

ad-m commented Jul 26, 2021

Mam wrażenie, że problem z uwierzytelnianiem został zaadresowany w #996 . Czego brakuje Ci w #996?

@MichalKarol
Copy link
Collaborator Author

W #996 mamy fejkowego oautha, natomiast nie pozwala nam to pracować z zewnętrznym serwerem developerskim dev.small-eod.siecobywatelska.pl (o ile wstanie :P). Plus dodanie opcji przeskoczenia na basic auth pozwoli nam na szybką zmianę pomiędzy użytkownikami i ich permissionami. Jeszcze jednym plusem jest możliowść pracy na jednym koncie na devie, zamiast ustawiać konto per developer. Ale tak po prawdzie głównie nie zauważyłem dodania fejkowego serwera OAuth,

@ad-m
Copy link
Member

ad-m commented Jul 26, 2021

@rwakulszowa możesz podzielić się opinią?

@ad-m
Copy link
Member

ad-m commented Jul 26, 2021

@MichalKarol, publiczne środowisko dev już działa w oparciu klasyczną infrastrukturę Stowarzyszenia. Uprzejmie proszę o uwagi.

@rwakulszowa
Copy link
Member

Cóż, dodanie fake oauth to mała zmiana, więc jeśli to rozwiązanie załatwia więcej problemów, to - jak dla mnie - możemy fake oauth szybko wycofać.

O ile dobrze rozumiem, rozwiązanie zaproponowane tutaj sprowadza się do podania danych logowania przy budowaniu projektu - tj. użytkownik albo jest zalogowany, albo nie, przez czas działania aplikacji. To może być problemem przy testowaniu #955. Mamy też trochę utrudnione testowanie całego flow autoryzacji (tj. wylogowany -> zalogowany -> refresh ..).

Głównym powodem dla którego dodałem fake-oauth był fakt, że prawdziwy OAuth jest trudny / niemożliwy do wywołania z localhost. Priorytetem było łatwe postawienie całej aplikacji przez docker-compose up.
O ile rozumiem (nie używałem dev.small-eod, więc mogę się mylić), na środowisku developerskim mamy skonfigurowane Google OAuth i można go używać. Przy dodawaniu fake-oauth raczej zakładałem, że na dev chcemy mieć środowisko jak najbliższe produkcji, a zmiany powinny być ograniczone tylko do aplikacji na localhost.

TLDR: sam skłaniałbym się raczej w stronę fake-oauth, bo wydaje mi się, że logika oparta na flagach podanych ze zmiennych środowiskowych wygląda mniej podejrzanie przy backendzie, ale nie jest to jakaś silna preferencja. Dopóki zaproponowane rozwiązanie pozwoli na łatwe testowanie na localhost, jest mi w zasadzie obojętne co będzie się działo pod spodem.

@@ -2,6 +2,11 @@ import { TokenResponse } from '@/services/definitions';
import { UsersService } from '@/services/users';
import SmallEodClient from '@/utils/sdk';

const isBasicAuth = Boolean(
// @ts-ignore
typeof USER !== 'undefined' && typeof PASSWORD !== 'undefined' && USER && PASSWORD,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Raczej nie ma scenariusza, w którym check USER && PASSWORD nie załatwi jednocześnie typeof /*jedno i drugie */ !== 'undefined'.

W ogóle nie brakuje tu, bo ja wiem, jakiejś deklaracji/importu?

@ad-m
Copy link
Member

ad-m commented Jul 26, 2021

@MichalKarol czy rozwiązanie z fake-oauth rozwiązuje twój przypadek?

@MichalKarol
Copy link
Collaborator Author

Zamykam PR jako że faktycznie wprowadza zmiany które nie są potrzebne z racji posiadania fake-oauth lokalnie. Natomiast widzę @ad-m że na dev jest jakiś błąd konfiguracji, bo dostaję:

Błąd 401: invalid_client
The OAuth client was not found.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants